Skip to content

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Jan 19, 2025

While MSVC is apparently fine using _AddressOfReturnAddress() without including intrin.h, Clang complains that the function is undefined. So we include the header, if _MSC_VER is defined, what is the case for MSVC and clang-cl, but not for some other compilers on Windows (e.g. GCC).


Note that this is related to 36857ab. It is indeed correct, that -fmodules declares some intrinsics (without -fmodules, hash_sha_ni.c and possibly others fail to build, even when I'm including intrin.h there). However, there are apparently no longer conflicting declarations in Clang (tested with 18.1.8), and without the include, I'm getting:

In file included from Zend\zend_constants.c:21:
In file included from Zend\zend_constants.h:23:
In file included from Zend\zend_globals.h:42:
Zend\zend_call_stack.h(46,9): error: call to undeclared library function '_AddressOfReturnAddress' with type 'void *(void)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   46 |         return _AddressOfReturnAddress();
      |                ^
Zend\zend_call_stack.h(46,9): note: include the header <intrin.h> or explicitly provide a declaration for '_AddressOfReturnAddress'
1 error generated.

PS: note that Clang does not (yet) support __builtin_frame_address().

While MSVC is apparently fine using `_AddressOfReturnAddress()` without
including intrin.h, Clang complains that the function is undefined.  So
we include the header, if `_MSC_VER` is defined, what is the case for
MSVC and clang-cl, but not for some other compilers on Windows (e.g.
GCC).
Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@cmb69 cmb69 merged commit ce53dab into php:master Jan 20, 2025
10 checks passed
@cmb69 cmb69 deleted the cmb/clang-intrin.h branch January 20, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants